Transform commands of pkg klass100 #1285

kylixs · 2020-07-01T12:35:10Z

include commands: classloader, dump, getstatic, jad, mc, ognl, redefine, sc, sm

…, mc, ognl, redefine, sc, sm

core/src/main/java/com/taobao/arthas/core/command/klass100/ClassLoaderCommand.java

core/src/main/java/com/taobao/arthas/core/command/klass100/DumpClassCommand.java

core/src/main/java/com/taobao/arthas/core/command/klass100/JadCommand.java

core/src/main/java/com/taobao/arthas/core/command/klass100/MemoryCompilerCommand.java

core/src/main/java/com/taobao/arthas/core/command/klass100/OgnlCommand.java

core/src/main/java/com/taobao/arthas/core/command/klass100/RedefineCommand.java

core/src/main/java/com/taobao/arthas/core/command/klass100/SearchClassCommand.java

core/src/main/java/com/taobao/arthas/core/command/klass100/SearchMethodCommand.java

core/src/main/java/com/taobao/arthas/core/command/model/ClassLoaderVO.java

core/src/main/java/com/taobao/arthas/core/command/model/ClassVO.java

core/src/main/java/com/taobao/arthas/core/command/model/SearchClassModel.java

…ocess.end() anywhere

kylixs · 2020-07-07T06:38:14Z

Arthas Command process() 原来的处理方式：

public void process(CommandProcess process){
	try {
        if (xxx) {
          process.end(-1, "xxxx" + t.getMessage());
          return;
        }
        if (yyy) {
           ...
          process.end();
          return;
        }
        // ...
    } catch (Throwable t) {
        process.end(-1, "process failure: " + t.getMessage());
    } finally {
        process.end();
    }
}

process.end() 或者process.end(0) 表示命令执行成功，process.end(code, message)表示命令执行失败，这个命令执行结果状态对于http api非常重要，客户端据此判断命令是否执行成功。
上面代码中 finally 中调用process.end() 的逻辑存在问题，可能是重复调用，也可能是错误调用：

如果处理过程曾经调用了process.end(code, message) ，在finally会再次调用process.end()引起理解混乱，前面设为错误状态，后面又设为成功状态（目前代码中有状态检查，只有第一次调用process.end(...)生效，第二次调用被忽略）；
如果不在finally 中调用process.end()，可能存在某些处理分支没有调用process.end()导致命令处理流程没有结束；
命令的子函数可能出现异常，没有调用process.end()但在外层finally块强制调用process.end()，会造成执行失败的命令返回执行成功状态；
比较难保证process.end()调用是有且唯一，代码编码过程要额外小心。

计划改为下面的处理方式：

public ExitStatus process(CommandProcess process){
	try {
        if (xxx) {
          return ExitStatus.failure(-1, "xxxx" + t.getMessage());
        }
        // ...
        return ExitStatus.success();
    } catch (Throwable t) {
        return ExitStatus.failure(-1, "process failure: " + t.getMessage());
    }
}

避免到处调用process.end()，这个方法使用很难保证正确，可能存在遗漏问题，改为返回ExitStatus可以在外层进行检查；
代码逻辑更直观，可以保证每个执行分支的ExitStatus是有且唯一，解决重复调用process.end()的问题
根据ExitStatus的值可以区分同步执行命令和异步执行的命令

kylixs · 2020-07-09T06:40:12Z

修改为：

public void process(CommandProcess process){
    try {
        if (xxx) {
          process.end(-1, "xxxx" + t.getMessage());
          return;
        }
        if (yyy) {
           ...
          process.end();
          return;
        }
        for ( .. ) {
            try {
                ...
            } catch (Exception e) {
                process.end(-1, "zzz" + t.getMessage());
                return;
            }
        }
         ...
        process.end();
    } catch (Throwable t) {
        process.end(-1, "process failure: " + t.getMessage());
    }
}

每个执行分支结束时必须要调用process.end() or process.end(code, msg)
如果代码执行抛出异常会被外层捕获，返回统一的错误码和提示信息
process.end() or process.end(code, msg) 后面应该加return ;语句结束执行，除非是在分支的最后面
finally 块中不能调用process.end()，也不能调用 process.appendResult(new RowAffectModel(affect));。因为在finally之前已经结束命令执行，不能再发送数据。

core/src/main/java/com/taobao/arthas/core/command/klass100/OgnlCommand.java

core/src/main/java/com/taobao/arthas/core/command/klass100/RedefineCommand.java

core/src/main/java/com/taobao/arthas/core/command/klass100/SearchMethodCommand.java

core/src/main/java/com/taobao/arthas/core/command/klass100/SearchClassCommand.java

core/src/main/java/com/taobao/arthas/core/command/model/SearchClassModel.java

core/src/main/java/com/taobao/arthas/core/util/CommandUtil.java

kylixs added 2 commits June 30, 2020 19:57

transform commands of pkg klass100: classloader, dump, getstatic, jad…

326f56f

…, mc, ognl, redefine, sc, sm

remove unused code

015a8c3

kylixs requested a review from hengyunabc July 1, 2020 12:35